Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve market upgrade messages + new switch #28812

Merged
merged 2 commits into from
Aug 31, 2017
Merged

Improve market upgrade messages + new switch #28812

merged 2 commits into from
Aug 31, 2017

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Aug 28, 2017

Description

Improve market upgrade messages.
Added switch to disable automatic app updates during occ upgrade.

Related Issue

Fixes #28734

Motivation and Context

How Has This Been Tested?

  • TEST: does not attempt to update apps if upgrade.automatic-app-update is set to false
  • TEST: attempts to update apps if upgrade.automatic-app-update is set to false
  • TEST: check message when occ upgrade with market app disabled: link to docs is there
  • TEST: check message when upgrading incompatible apps not present on marketplace: link to docs is there
  • TEST: market app auto-disables if "has_internet_connection" is false during occ ugprade.
  • TEST: if "has_internet_connection" is false and incompatible apps detected, stop update and mention those

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Improve market upgrade messages.
Added switch to disable automatic app updates during occ upgrade.
Even if no internet connection is available, we still need to verify
that there are no incompatible apps on upgrade and show an appropriate
message.
@PVince81
Copy link
Contributor Author

upgrade-flow

@pmaier1

@patrickjahns patrickjahns self-requested a review August 31, 2017 09:04
@@ -114,7 +114,7 @@ public function run(IOutput $output) {
$link = $this->defaults->buildDocLinkToKey('admin-marketplace-apps');
$output->info('No internet connection available - no app updates will be taken from the marketplace.');
$output->info("How to update apps in such situation please see $link");
return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will likely change the behaviour. Is it fine?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is intended. We should not return directly because even if we don't attempt contacting the marketplace, we should still check if there is any incompatible app or app missing code blocking the update. Returning here would bypass the check and result in a broken OC in such scenarios.

Copy link
Member

@jvillafanez jvillafanez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine, not tested.

@PVince81 PVince81 merged commit da5030c into master Aug 31, 2017
@PVince81 PVince81 deleted the update-messages branch August 31, 2017 12:42
@PVince81
Copy link
Contributor Author

stable10: #28871

@lock
Copy link

lock bot commented Aug 2, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Core upgrade process: Feedback for administrators
3 participants